This is reversed I think...if the iceberg CRS is "", the GeoArrow CRS is "OGC:CRS84" (unless NewMetadata().

There is one special case here: "srid:0" maps to an omitted GeoArrow CRS.

I believe there is one special case here: if the Iceberg CRS is projjson:<table property>, the CRS is the JSON object in that table property field. You should probably either support that or error with a note saying that it's not supported.

If the GeoArrow CRS is omitted from the extension metadata, the Parquet equivalent is "srid:0". If the GeoArrow CRS is "OGC:CRS84" or "EPSG:4326", the canonical Parquet CRS is "omitted" (i.e., default). I believe the Parquet and Iceberg CRS definitions are in sync now but it's worth double checking.

If the CRSType is PROJJSON, this is a important case to handle (because most GeoArrow producers produce that). You can convert most of these to authority:code by looking for the id member that looks like this "crs": {..., "id":{"authority": "OGC", "code": "CRS84"} or this "crs": {..., "id":{"authority": "EPSG", "code": 3857}. It would also be a good idea to convert the two "lonlat" CRSes ("EPSG:4326" and "OGC:CRS84") to the Iceberg "default" canonically.

When you can't extract an authority:code, this would need to be written to a table property and written to the CRS field as (projjson:<the table property name>). Probably easier to error for that case for now.

EPSG:4326 and OGC:CRS84 aren't the same CRS — they have opposite axis order (CRS84 is lon/lat, EPSG:4326 is lat/lon). Collapsing them here means a geometry("EPSG:4326") field — which is exactly what PyIceberg emits via ga.wkb().with_crs("EPSG:4326") — reads back as geometry("OGC:CRS84"), so the schema silently changes on the read side.

The write path doesn't share the collapse either: icebergCRSToGeoArrowMetadata emits EPSG:4326 verbatim, so GeometryType{crs:"EPSG:4326"} → Arrow → GeometryType{crs:"OGC:CRS84"} and the two aren't Equals.

I'd drop the EPSG:4326 case here and treat it as a distinct authority code. If we genuinely want them unified, it has to be symmetric — normalize on both the write and read sides so the round trip is stable — and it shouldn't live silently at the schema layer. Either way a round-trip regression test for EPSG:4326 specifically would lock it down. wdyt?

EPSG:4326 and OGC:CRS84 aren't the same CRS

They are for the purposes of the GeoArrow and Parquet specifications, which explicitly define the axis order for these cases

I believe that a round trip regression test is definitely in order for this behavior.

@paleolimbot, is the asymmetric behavior here expected or should we collapse "EPSG:4326" at the write level too? (i.e. add a conditional in icebergCRSToGeoArrowMetadata)

The ideal behaviour is that all of these become the Iceberg default CRS:

• "crs": "EPSG:4326" and "crs": "OGC:CRS84" (case insensitive)
• "crs": {..., "id":{"authority": "OGC", "code": "CRS84"}
• "crs": {..., "id":{"authority": "EPSG", "code": 4326}
• • "crs": {..., "id":{"authority": "EPSG", "code": "4326"}

When reading an iceberg default CRS to GeoArrow, emit "crs": "OGC:CRS84". I think your PR is currently missing this case.

This asymmetry is helpful to improve the compatibility of iceberg tables (e.g., for readers that can't or don't want to understand CRSes and only handle the default case).

This panics on projjson, but the symmetric read function geoArrowCRSToIcebergCRS returns a clean error for the same case. The asymmetry is the problem: VisitGeometry/VisitGeography have no error return and nothing recovers, so a GeometryType with a projjson: CRS detonates the whole conversion at TypeToArrowType rather than surfacing an error.

I'd return an error here and thread it through the same way the read path does, so a projjson CRS fails gracefully instead of crashing the process.

The %v on err drops it out of the wrapped chain, so errors.Is/errors.As against the underlying error won't work downstream. I'd use %w for both:

panic(fmt.Errorf("%w: converting geoarrow metadata: %w", iceberg.ErrInvalidSchema, err))

CRSTypeAuthorityCode and CRSTypeWKT22019 both fall into default and get returned as-is. For authority_code that's almost right but loses the type tag; for WKT2:2019 it hands a multi-KB WKT2 blob straight back as the Iceberg CRS string, which isn't a valid CRS identifier.

I'd add an explicit CRSTypeAuthorityCode case, and for CRSTypeWKT22019 either error like projjson does or reduce it to the authority code — whichever we pick, a test for each so the behavior is pinned.

The CRSType, if it's coming from JSON, is just a hint, is not required, and is often absent. Here you probably want to:

• Check if crs is a JSON object. If it is, check the id member and paste together crs["id"]["authority"], :, and crs["id"]["code"]. If any of those are missing, error for an unsupported CRS>
• Check if crs is a string. If it's shorter than 32 characters, let it through verbatim. There's no official restriction on allowed characters in authorities or codes but the length check should reject anything questionable.

For an authority-code CRS like EPSG:4326 this emits {"crs":"EPSG:4326"} with no crs_type, but the GeoArrow spec wants crs_type: "authority_code" for AUTHORITY:CODE strings. Without it the encoding is ambiguous, and combined with the collapse in geoArrowCRSToIcebergCRS it's the other half of the EPSG:4326 round-trip break.

I'd set CRSType: geoarrow.CRSTypeAuthorityCode when the CRS matches the authority:code shape, so the write side is unambiguous and pairs cleanly with an explicit authority_code read case.

"crs_type": "authority_code" is optional and no consumer actually requires this, but it is polite to include it (I put this in the Arrow C++ export path).

errcheck is enabled outside _test.go, so the discarded errors here and at the json.Marshal(crs) below will fail CI. Marshaling a string can't actually fail, but the linter doesn't know that — I'd either build the raw JSON without the error return (e.g. json.RawMessage(strconv.AppendQuote(nil, id))) or add a //nolint with a one-line why.

A bare ga.wkb() field with no CRS — the PyIceberg default and what older clients emit — reads back as geometry("srid:0") rather than geometry(), which isn't Equals to a default-CRS geometry.

Separately, an empty meta.CRS with a non-empty CRSType skips this early return and falls into the switch, so CRSTypeSRID yields "srid:" with an empty id and GeometryTypeOf accepts it silently. I'd return the OGC:CRS84 default for the bare case and guard the empty-CRS-with-CRSType combination, with a test for bare geoarrow.wkb.

bare ga.wkb() field with no CRS — the PyIceberg default and what older clients emit — reads back as geometry("srid:0") rather than geometry(), which isn't Equals to a default-CRS geometry.

This is the correct behaviour: the GeoArrow default does not equal the Iceberg default. PyIceberg is probably wrong here.

Separately, an empty meta.CRS with a non-empty CRSType skips this early return

This should be fixed...the CRSType can actually just be ignored for the purposes of this function (it's purely a hint)

Small thing while we're here: strings.ToLower(crs) runs twice (here and for the projjson check) and the slice crs[len("srid:"):] indexes the original-case string after a lowercased prefix check. I'd hoist lower := strings.ToLower(crs) once and reuse it. Also worth noting the EPSG:4326 match in the read function is case-sensitive, so epsg:4326 falls through — strings.EqualFold would close that.

EPSG:4326 and OGC:CRS84 aren't the same CRS

They are for the purposes of the GeoArrow and Parquet specifications, which explicitly define the axis order for these cases

bare ga.wkb() field with no CRS — the PyIceberg default and what older clients emit — reads back as geometry("srid:0") rather than geometry(), which isn't Equals to a default-CRS geometry.

This is the correct behaviour: the GeoArrow default does not equal the Iceberg default. PyIceberg is probably wrong here.

Separately, an empty meta.CRS with a non-empty CRSType skips this early return

This should be fixed...the CRSType can actually just be ignored for the purposes of this function (it's purely a hint)

The CRSType, if it's coming from JSON, is just a hint, is not required, and is often absent. Here you probably want to:

• Check if crs is a JSON object. If it is, check the id member and paste together crs["id"]["authority"], :, and crs["id"]["code"]. If any of those are missing, error for an unsupported CRS>
• Check if crs is a string. If it's shorter than 32 characters, let it through verbatim. There's no official restriction on allowed characters in authorities or codes but the length check should reject anything questionable.

"crs_type": "authority_code" is optional and no consumer actually requires this, but it is polite to include it (I put this in the Arrow C++ export path).

The id is sliced out of the lowercased string, so a non-numeric SRID gets silently case-folded: srid:MyDB goes in, mydb gets marshalled, and it round-trips back as srid:mydb. Numeric SRIDs are fine, which is why it's latent.

I'd slice the original crs instead — id := crs[len("srid:"):] — and keep the HasPrefix check on lowerCRS.

The iceberg_to_arrow_round_trip loop only iterates typeCases, which has geomSRID0 but no non-zero SRID, so the common path isn't exercised either. Adding a srid:4326 case to typeCases covers the happy path and would have caught this — worth pairing the fix with that test case.

good catch, will update the tests to include this.

This is package internal, and WKTToWKB's only callers are _test.go files, but go-geom is now a direct require in go.mod — so every consumer of table/internal (scanner, DV writer) transitively pulls it into the production module graph.

I'd move this into a _test.go file in package internal_test (the test already lives there) or a small testutil package, so go-geom stays test-only. Non-blocking from my side, and I haven't seen it raised on the threads — if the maintainers are fine carrying it that's a reasonable call — but I'd lean toward keeping it out of the production deps.

A small cleanup cluster in this function:

The if len(meta.CRS) > 0 wrappers inside both this checkCRSString arm and the checkCRSSJSON arm below are dead — the guard at the top of the function already returns on empty, and the check* funcs only return true for non-empty input (staticcheck SA9003). I'd drop both wrappers.

On the write path (line 1984), strings.EqualFold(lowerCRS, "EPSG:4326") runs a case-fold against an already-lowercased string — lowerCRS == "epsg:4326" is enough.

And in the JSON-object code parsing (line 1923), || codeNum.String() == "" is unreachable — a successful json.Number unmarshal is never empty. I'd drop just that clause; the trailing if code == "" is reachable for {"code":""}, so leave that one.

geo_codec_test.go is all happy-path — no invalid-WKT case. I'd add a wantErr row (e.g. "not wkt") so the error path is exercised. While we're here, the wkt.Unmarshal / wkb.Marshal errors return bare; wrapping them (fmt.Errorf("parse WKT: %w", err)) makes failures legible at the call site.

Minor: checkCRSSJSON has a double S — rename to checkCRSJSON at the def and the call site.

Good catch!

I think this comment is misleading (planar geography is not a thing, and this is not a hint, it affects correctness).